Your call as to what you incorporate. :-)

I might change:

"Maximum number of virtual machines (VMs) or disks per plan that can be migrated simultaneously."
To:
"Maximum number of virtual machines (VMs) or disks per plan that can migrate simultaneously."

(---unless “can be” means that you may decide not to and have that option)

"The maximum number of disks that can be transferred simultaneously."
To:
"The maximum number of disks that can transfer simultaneously."

"The maximum number of VMs that can be migrated simultaneously."
To:
"The maximum number of VMs that can migrate simultaneously."

"…can be transferred simultaneously. In these migrations, the disks are migrated in parallel. This means that if the combined number of disks that you want to migrate is greater than the value of the setting, additional disks wait until the queue is free without regard for whether a VM has completely finished being migrated."
To:
"…can be transferred simultaneously. In these migrations, the disks migrate in parallel. If the combined number of disks that you want to migrate is greater than the value of the setting, additional disks migrate only once the queue is free, without regard for whether a VM has finished migrating."

"Once any of them has migrated, the 16th disk can be migrated, whether or not all the disks on VM A and VM B have finished migrating."
To:
"Once any of them has migrated, the 16th disk migrates, whether or not all the disks on VM A and VM B have finished migrating."

(-- unless “can be migrated” means that it’s an option to migrate the disk)

@mnecas @sgratch Please review this PR. Thanks.

@RichardHoch @mnecas
Isn't it worth renaming the parameter to reflect its meaning to something like:
Max concurrent virtual machine or disks migrations?

@sgratch Changing the name of the parameter is up to @mnecas as far as I am concerned. But if we do it, I would suggest controller_max_vm_disk_inflight and Max concurrent virtual machine or disk migrations [drop the "s" in "disks" -- English is weird]. Does the PR look OK otherwise?

Changing the name of the parameter is up to @mnecas as far as I am concerned. But if we do it, I would suggest controller_max_vm_disk_inflight and Max concurrent virtual machine or disk migrations [drop the "s" in "disks" -- English is weird].

Sounds good to me to change it. @mnecas ?

Changing the name of the parameter is up to @mnecas as far as I am concerned. But if we do it, I would suggest controller_max_vm_disk_inflight and Max concurrent virtual machine or disk migrations [drop the "s" in "disks" -- English is weird].

Sounds good to me to change it. @mnecas ?

I am a bit afraid that we would lose the previous settings during the upgrade.
I don't want to change it to the "or" as we might even change the logic to always transfer disks.

Changing the name of the parameter is up to @mnecas as far as I am concerned. But if we do it, I would suggest controller_max_vm_disk_inflight and Max concurrent virtual machine or disk migrations [drop the "s" in "disks" -- English is weird].

Sounds good to me to change it. @mnecas ?

I am a bit afraid that we would lose the previous settings during the upgrade. I don't want to change it to the "or" as we might even change the logic to always transfer disks.

OK, so let's leave it as is for now.

@sgratch @mnecas I made the change Sharon suggested. Please review this PR again. Thanks.

@RichardHoch did you miss those 2 comments ? :)
#580 (comment)
#580 (comment),

@sgratch @mnecas I made some more changes to the PR. Please review them. Thanks.
@sgratch I think I included both your changes -- let me know if I missed something.

The 'controller_max_vm_inflight' parameter is related to simultaneous migration.
While using VMware as the source provider:
COLD migration - VMs per ESXi host that can migrate simultaneously
WARM migration - Disks per ESXi host that can migrate simultaneously

@sgratch I think I included both your changes -- let me know if I missed something.

LGTM

@anarnold97

LGTM

@duduvaa You replied LGTM to Andy's suggestion, but what about cold remote? Martin commented "For cold remote we use the CNV CDI so the max in flight references the number of disks" Does that fit your test results?
That is, what do you say to:

• For VMware simultaneous migration:

** Cold migration:

*** To local {virt}: VMs for each ESXi host that can migrate simultaneously
*** To remote {virt}: Disks for each ESXi host that can migrate simultaneously

** Warm migration: Disks for each ESXi host that can migrate simultaneously

@RichardHoch ,
I'm trying to set my environment for remote cold migration.
I'll update

@RichardHoch ,

I tested cold migration to remote env.
As Martin said, "max_vm_flight" references the number of disks per ESXi host.

Richard, as you wrote
** Cold migration:

*** To local {virt}: VMs for each ESXi host that can migrate simultaneously
*** To remote {virt}: Disks for each ESXi host that can migrate simultaneously

@mnecas @duduvaa Please review. What about OpenShift Virtualization as a source? VMs or disks?

@RichardHoch I believe you asked about Redhat Virtualization/ Openstack not OCP-V

@RichardHoch I believe you asked about Redhat Virtualization/ Openstack not OCP-V

@duduvaa Actually, I did mean OpenShift Virtualization, aka CNV, because it's a source provider, but we didn't define whether the parameter applies to disks or VMs when CNV is the source provider.

@RichardHoch I believe you asked about Redhat Virtualization/ Openstack not OCP-V

@duduvaa Actually, I did mean OpenShift Virtualization, aka CNV, because it's a source provider, but we didn't define whether the parameter applies to disks or VMs when CNV is the source provider.

@fabiand , @mnecas
Could you relate to Richard's comment?

OCP can be both - src + dst.

@mnecas mentioned that max inflight for CNV is about inflight VMs.

We are using the VirtualMachineExport to transfer the VMs between clusters/providers.
https://kubevirt.io/user-guide/storage/export_api/

@mnecas I have added the information for CNV as provider. Please review the PR again. Thanks.

@mnecas I made the changes you suggested. Please review this PR again.

@anarnold97: Please give this PR another review.

https://file.corp.redhat.com/rhoch/max_concur_vm/html-single/#configuring-mtv-operator_mtv [first bullet in list and first item in Table 3.1]

In the preview, in the first item in table 3.1, middle column, the lead-in is bold. Is that what you intended? Also, the left-hand column - do you want those to be all bold? The left-hand column of 4.1 is not bold (I know they are “label” versus “Setting”, so perhaps our standards call for this.)

Just above 3.1, “Spec: label: value” looks large. If this follows our standards, cool. Just wanted to point it out.

https://file.corp.redhat.com/rhoch/max_concur_vm/html-single/#max-concurrent-vms_mtv [new section]

This bullet point includes VM in italics; the others do not. “For OVA migrations, the label specifies the maximum number of VMs that MTV…”

https://file.corp.redhat.com/rhoch/max_concur_vm/html-single/#mtv-settings_mtv [first item in Table 4.1]

That first item in Table 4.1 also includes a bold lead-in for the middle column, but not for subsequent entries into the middle column. Also, at the end of the first column, “+ See Configuring the…” The +See caught my eye.

@nunzy1 1. FYI -- In the previews, items with backticks appear to be bolded, but they aren't in production. You're right about "Varies with provider," which i fixed.
2. The italicization of VMs in the section is intended; "disks" is italicized in the bullet above. I think that "disks" vs. "VMs" should be italicized. Do you think that's overkill or that the emphasis is lost in the text? Either reason would be a good reason to drop the italics -- what do you think?
3. Fixed both. Thanks.